Skip to content

Revive for modern Node.js: Node-API, vendored Lua, tests, CI - #1

Merged
mschmicking merged 5 commits into
masterfrom
revive-2.0
Aug 7, 2026
Merged

Revive for modern Node.js: Node-API, vendored Lua, tests, CI#1
mschmicking merged 5 commits into
masterfrom
revive-2.0

Conversation

@mschmicking

@mschmicking mschmicking commented Aug 6, 2026

Copy link
Copy Markdown
Owner

Why

The package did not build. Two independent reasons:

  1. NAN no longer compiles. It tracks V8's unstable C++ API. On Node 26 the build fails with 20 errors inside nan.h itself — v8::String::REPLACE_INVALID_UTF8 and Isolate::IdleNotificationDeadline were removed, and GetAlignedPointerFromInternalField changed signature. No change to our own code could have fixed this.
  2. It could not install on most machines. maclualib/lib/liblua.a was an x86_64-only Mach-O, so every Apple Silicon Mac failed. Linux hardcoded /usr/local/lib/libluajit-5.1.so, which the user had to build by hand. No ARM64 support anywhere. Only Windows x64 worked out of the box.

Goal: get it working, and back on npm so people can actually use Lua from Node.

What changed

Binding layer: NAN → Node-API (node-addon-api). Node-API is ABI-stable, so one build keeps working across future Node majors rather than needing a release every time Node ships one. This is the change that stops the maintenance treadmill.

Lua engine: vendored from source. Lua 5.1.5 (29 C files) and LuaFileSystem 1.8.0 now compile into the addon. maclualib/ and win64luajit/ are deleted. Builds on every platform and architecture, with no system dependency.

  • LuaJIT is dropped. Only Windows ever had it — macOS already shipped stock Lua 5.1.5 despite what the README claimed. In exchange, Apple Silicon and ARM64 Linux work at all.
  • require('lfs') now works everywhere. It used to be a Windows-only prebuilt DLL loaded through an LUA_CPATH hack in index.js. It is now compiled in and registered via package.preload.

Staying on the Lua 5.1 line is deliberate: LUA_GLOBALSINDEX is part of the public API here and 5.2+ removed it.

Bug fixes

All user-visible, which is why this is a major version.

Fix Impact
SetField pushed its key as the value Every SetField wrote the field name into the field
SetField did not resolve a relative index before pushing SetField(-1, ...) indexed into the value → unprotected Lua error → aborted the process
LoadFile/LoadString were bound to the DoFile/DoString handlers They executed instead of only compiling; the real implementations were dead code
AddPackagePath appended to package.path with no ; separator Corrupted the last entry, so require usually failed — the lua_require example never worked
AddPackagePath interpolated the path into generated Lua source A path containing a quote broke out of the string literal
Booleans converted with Nan::New((int)...) Arrived in JS as 1/0 instead of true/false
Push used lua_pushinteger Push(3.5) truncated to 3
Table conversion used a hardcoded relative index Only worked when the table sat on top of the stack
get_str malloc'd and never freed Leaked on every string argument
Six sprintf calls into a fixed char buf[1024] Arbitrary-length Lua error messages overflowed a stack buffer
~LuaState never called lua_close; double Close Leaked the interpreter; second Close was a use-after-free

Close is now idempotent and any later use of the state throws. SetField/GetField reject non-table targets rather than letting Lua abort the process.

Registered callbacks are now found through a closure upvalue instead of a global singleton — this was less code than porting the singleton across, and it means two LuaState instances no longer clash.

Tests and CI

  • 62-case suite on node:test (zero new dependencies), with explicit regression tests pinning each fix above.
  • GitHub Actions matrix: Linux / macOS / Windows × Node 20, 22, 24. macos-latest is arm64 — precisely the configuration that could not build before.

Verification

Locally on aarch64 Linux: 62/62 tests pass, all three examples run (including lua_lfs, which has never worked off Windows), and the packed tarball — 66 files, no build/, both third-party licenses included — unpacks, compiles and passes a smoke test in a clean directory.

Windows and macOS could not be tested locally. The CI matrix is the gate; do not publish until all nine cells are green.

Notes for the reviewer

  • binding.gyp defaults android_ndk_path, which Node's common.gypi dereferences but never defines. Harmless off Android, and it makes Termux builds work.
  • .npmignore is replaced by a files allowlist in package.json — getting this wrong is the likeliest way to publish a broken 2.0.0, so the tarball contents are asserted in CI via npm pack --dry-run.
  • homepage/repository pointed at the old 0x7878 handle and rendered as broken links on npm. Fixed, and bugs added.
  • Unrelated to this PR: npm 12 blocks install scripts by default, so users will need npm install-scripts approve node-lua-runner. This affects every native addon; the only real escape is shipping prebuilt binaries, deliberately left out of scope.

🤖 Generated with Claude Code

mschmicking and others added 5 commits August 6, 2026 15:13
@mschmicking @claude
Revive for modern Node.js: Node-API, vendored Lua, tests, CI
0073e1b 
The package no longer built. NAN tracks V8's unstable C++ API and fails to
compile on current Node (20 errors inside nan.h itself on Node 26), and the
prebuilt Lua libraries only covered Windows x64 and Intel macOS, so Apple
Silicon and ARM64 Linux could not install at all.

- Migrate the binding from NAN to Node-API via node-addon-api. Node-API is
  ABI-stable, so a build keeps working across future Node majors.
- Vendor Lua 5.1.5 and LuaFileSystem 1.8.0 and compile them into the addon.
  Removes maclualib/ and win64luajit/, works on every platform and arch, and
  makes require('lfs') available outside Windows for the first time.
- Drop LuaJIT. Only Windows had it; macOS already shipped stock Lua 5.1.5.

Bug fixes, all user-visible, hence the major version:

- SetField pushed its key argument as the value, and did not resolve a relative
  index before pushing, which put Lua in an unprotected error and aborted the
  process.
- LoadFile/LoadString were bound to the DoFile/DoString handlers, so they
  executed instead of only compiling.
- Lua booleans converted to the numbers 1 and 0 instead of true and false.
- Push truncated numbers through lua_pushinteger, turning 3.5 into 3.
- AddPackagePath appended to package.path without a separator, corrupting the
  last entry so require usually failed, and interpolated the path into
  generated Lua source.
- Table conversion used a hardcoded relative stack index and only worked when
  the table sat on top of the stack.
- get_str malloc'd on every string argument and never freed.
- Six sprintf calls formatted arbitrary-length Lua error messages into a fixed
  1024-byte stack buffer.
- ~LuaState never called lua_close, and a second Close was a use-after-free.
  Close is now idempotent and later calls throw instead of crashing.
- Registered functions are found via a closure upvalue rather than a global
  singleton, so separate LuaState instances no longer clash.

Adds a 62-case test suite on node:test and a GitHub Actions matrix covering
Linux, macOS and Windows across Node 20, 22 and 24.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@mschmicking @claude
Fix Windows CI on Node 20 and 22 by using a current node-gyp
accafa7 
The windows-latest image ships Visual Studio 18. The node-gyp bundled with
npm on Node 20 and 22 does not recognise it and fails configure with
'find VS unknown version "undefined"'; Node 24 already carries a new enough
node-gyp, which is why only those two cells failed.

Install node-gyp@latest on Windows runners and point npm at it. Also document
the same workaround for users, who hit this whenever they pair Node 20 or 22
with Visual Studio 2026.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@mschmicking @claude
Override node-gyp via npm_config_node_gyp, not npm config
4c68852 
'npm config set node_gyp' was removed in npm 11 and now errors with
'`node_gyp` is not a valid npm option', which broke every Windows cell
including Node 24, that had been passing.

npm's bundled node-gyp shim defers to $npm_config_node_gyp when set, so
export that through GITHUB_ENV instead. Correct the README workaround the
same way.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@mschmicking @claude
Pin node-gyp 12 on Windows instead of trusting @latest
c2ad63b 
The previous step installed node-gyp@latest, but npm resolves that per Node
major through engines: Node 24 got 12.3.0 and built fine, while Node 20 and 22
got 11.5.0 and still failed. 11.x is exactly the version that cannot detect
Visual Studio 18 — its vswhere probe overflows the child-process stdio buffer
and it reports 'unknown version "undefined"'.

Pin node-gyp@12, install it under RUNNER_TEMP so the path is deterministic
rather than depending on where the global prefix lands, and print the resolved
version so the log shows which one actually ran. README workaround updated to
name @12 explicitly for the same reason.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@mschmicking @claude
Build with an explicit node-gyp instead of npm's bundled copy
4dc9ede 
Modern npm ignores $npm_config_node_gyp, so the previous two attempts to
redirect it never took effect. The evidence: Node 22 reported node-gyp 11.5.0
whether @latest was installed globally or @12 was installed to a pinned path,
and Node 24 reported 12.3.0 in both cases. Those are just the versions npm
bundles per Node major, and 11.x is the one that cannot detect Visual Studio 18.

Stop trying to redirect npm. Install dependencies with --ignore-scripts and
invoke a pinned node-gyp 12 directly, which is deterministic across every cell.
Also removes a stale duplicated comment block left by the earlier edit.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@mschmicking
mschmicking merged commit 96cc357 into master Aug 7, 2026
9 checks passed
@mschmicking
mschmicking deleted the revive-2.0 branch August 7, 2026 21:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@mschmicking